Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat [188128]: Set up react project #36

Closed
wants to merge 18 commits into from

Conversation

NoraKri
Copy link
Contributor

@NoraKri NoraKri commented Nov 6, 2024

User Story 188128

Set up React project
A react project has been set up for the frontend using Vite.

Port functionality from vanilla JavaScript
Functionality for contact with the triplestore and highlighting has been moved over to React. This functionality is split up into different files for readability. The P&ID is displayed in a component named Diagram.

Update RunGuide.md
The steps to run the project has changed. RunGuide has been updated.

AB#188128

Copy link
Contributor

@daghovland daghovland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refactoring into react seems to give a more maintainable and extendable code base. The issue with lacking change in highlightling when clicking pipes should be fixed, the rest of the comments are suggestions, I think

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest not committing this, rather putting a link or info about where to find it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it be that this one is not used, just the one in www?

</g>
width="3.43931mm" height="17.4611mm" viewBox="-0.40625 -2.03125 0.8125 4.125">
<defs vector-effect="non-scaling-stroke" />
<g>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Jeg tror jeg likte den gamle formateringa best, men det er jo virkelig ikke viktig.

- Add the node to the boundary class, and remove it from the internal class.
- Update rdfox, insert the following triple :nodeId a :boundary .
- Update rdfox, delete the following triple :nodeId a :insideBoundary .
- Ctrl+left click to select or deselect the internal component
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Ctrl+left click to select or deselect the internal component
- Shift+left click to select or deselect the internal component

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When a pipe is selected as bounday, the highligthing of completion package/inside is not updated. To reproduce, select the tank as internal, and two pipes as part of the boundary

www/src/App.css Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove css that is not in use please

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these changes only formatting? If so, I would suggest to revert them in this PR and rather do a separate PR with only the formatting

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this file be removed?

let resultInside = parseNodeIds(await queryTripleStore(queryInside) as string);
const resultBoundary = parseNodeIds(await queryTripleStore(queryBoundary) as string);

if (resultInside.length > 0 || resultBoundary.length > 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove commented out code

@NoraKri NoraKri closed this Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants